Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[UT] [Portsyncd] Added Unit Tests for portsyncd #2297

Merged
merged 29 commits into from
Sep 1, 2022
Merged

[UT] [Portsyncd] Added Unit Tests for portsyncd #2297

merged 29 commits into from
Sep 1, 2022

Conversation

vivekrnv
Copy link
Contributor

@vivekrnv vivekrnv commented May 27, 2022 •
edited
Loading

What I did

Added Unit tests for portsyncd module:

  • Create a new make target under mock_tests/Makefile.am to include a target for linksync.cpp
  • Mocked a rtnl_link object for testing onMsg method of Linksync.cpp
  • Used/updated the existing mock infra to allow for testing.

Why I did it

To increase code/feature coverage

How I verified it

vkarri@e4bc0a3e5630:/sonic$ make -f slave.mk target/debs/bullseye/swss_1.0.0_amd64.deb
..........
make[3]: Entering directory '/sonic/src/sonic-swss/tests'
Making check in mock_tests
make[4]: Entering directory '/sonic/src/sonic-swss/tests/mock_tests'
make  check-TESTS
make[5]: Entering directory '/sonic/src/sonic-swss/tests/mock_tests'
make[6]: Entering directory '/sonic/src/sonic-swss/tests/mock_tests'
PASS: tests_portsyncd
PASS: tests_intfmgrd
PASS: tests
============================================================================
Testsuite summary for sonic-swss 1.0
============================================================================
# TOTAL: 3
# PASS:  3
# SKIP:  0
# XFAIL: 0
# FAIL:  0
# XPASS: 0
# ERROR: 0
============================================================================


vkarri@4897dcba242c:/sonic/src/sonic-swss/tests/mock_tests$ make tests_portsyncd 
.............
..............
..............
vkarri@5e69834fbb01:/sonic/src/sonic-swss/tests/mock_tests$ ./tests_portsyncd 
Running main() from /build/googletest-YnT0O3/googletest-1.10.0.20201025/googletest/src/gtest_main.cc
[==========] Running 6 tests from 1 test suite.
[----------] Global test environment set-up.
[----------] 6 tests from PortSyncdTest
[ RUN      ] PortSyncdTest.test_linkSyncInit
[       OK ] PortSyncdTest.test_linkSyncInit (0 ms)
[ RUN      ] PortSyncdTest.test_cacheOldIfaces
[       OK ] PortSyncdTest.test_cacheOldIfaces (0 ms)
[ RUN      ] PortSyncdTest.test_onMsgNewLink
[       OK ] PortSyncdTest.test_onMsgNewLink (0 ms)
[ RUN      ] PortSyncdTest.test_onMsgDelLink
[       OK ] PortSyncdTest.test_onMsgDelLink (0 ms)
[ RUN      ] PortSyncdTest.test_onMsgMgmtIface
[       OK ] PortSyncdTest.test_onMsgMgmtIface (0 ms)
[ RUN      ] PortSyncdTest.test_onMsgIgnoreOldNetDev
[       OK ] PortSyncdTest.test_onMsgIgnoreOldNetDev (0 ms)
[----------] 6 tests from PortSyncdTest (1 ms total)

[----------] Global test environment tear-down
[==========] 6 tests from 1 test suite ran. (1 ms total)
[  PASSED  ] 6 tests.

Details if related

vivekrnv and others added 18 commits April 27, 2022 18:38
@vivekrnv
Refactored portsyncd structure to improve testability
1c6cd64 
Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
@vivekrnv
Merge branch 'Azure:master' into ut_infra_improv
0ce3d92
@vivekrnv
Skeleton for portsyncd ut added
7395544 
Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
@vivekrnv
Merge branch 'ut_infra_improv' of https://github.com/vivekreddynv/son…
b98badf 
…ic-swss into ut_infra_improv
@vivekrnv
Minor update
dc384cd 
Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
@vivekrnv
Class Init and reading from cfg_db ut's added
bc7528a 
Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
@vivekrnv
Added missing comment
e052e20 
Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
@vivekrnv
onMsg test added
e267fd9 
Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
@vivekrnv
2 new test cases added
67a086d 
Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
@vivekrnv
Another case added
64a8ee1 
Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
@vivekrnv
Push infra modifications
6909333 
Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
@vivekrnv
Moved common methods/ds to a different file
7f26e83 
Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
@vivekrnv
bullseye link issue handled
0402330 
Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
@vivekrnv
Merge branch 'master' of https://github.com/Azure/sonic-swss into ut_…
0523b8c 
…infra_improv
@vivekrnv
Update UT to include ASAN changes
8a3c138 
Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
@vivekrnv
Restored original del func on mock_table
dfcba64 
Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
@vivekrnv
Refactored mock shell cmd
1bc2d97 
Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
@vivekrnv
Minor fix
6134e80 
Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
@vivekrnv vivekrnv requested a review from prsunny as a code owner May 27, 2022 06:20
@liat-grozovik
Copy link
Collaborator

/azp run Azure.sonic-swss

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@vivekrnv
Copy link
Contributor Author

/azpw run Azure.sonic-swss

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-swss

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

keboliu
keboliu previously approved these changes Jun 1, 2022
View reviewed changes
@vivekrnv
Copy link
Contributor Author

vivekrnv commented Jun 8, 2022

/azpw run Azure.sonic-swss

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-swss

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@vivekrnv
Copy link
Contributor Author

/azpw run Azure.sonic-swss

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-swss

@vivekrnv vivekrnv requested a review from keboliu August 2, 2022 08:15
@vivekrnv
Copy link
Contributor Author

vivekrnv commented Aug 2, 2022

@vivekrnv could you please handle conflicts and followup on checkers?

Handled. @prsunny can you please review

@vivekrnv
p4orch tests updated
98b6358 
Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
@prsunny
Copy link
Collaborator

prsunny commented Aug 4, 2022

i dont think we should combine refactoring and portsyncd changes together. Can you please have this PR only for the portsyncd?

@vivekrnv
Copy link
Contributor Author

vivekrnv commented Aug 4, 2022

i dont think we should combine refactoring and portsyncd changes together. Can you please have this PR only for the portsyncd?

Will update

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Aug 5, 2022 •
edited
Loading

CLA Signed

The committers listed above are authorized under a signed CLA.

@vivekrnv
Copy link
Contributor Author

vivekrnv commented Aug 5, 2022

i dont think we should combine refactoring and portsyncd changes together. Can you please have this PR only for the portsyncd?

updated

@vivekrnv
Copy link
Contributor Author

vivekrnv commented Aug 8, 2022

/easycla

@vivekrnv
Copy link
Contributor Author

vivekrnv commented Aug 8, 2022

/azpw run Azure.sonic-swss

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-swss

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@vivekrnv
Copy link
Contributor Author

vivekrnv commented Aug 8, 2022

/azpw run Azure.sonic-swss

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-swss

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

prsunny
prsunny reviewed Aug 12, 2022
View reviewed changes
portsyncd/portsyncd_helper.cpp Outdated
@@ -0,0 +1,64 @@
#include "portsyncd/linksync.h"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we discussed it last time, please move this to portsyncd itself. I don't see a reason why it has to be part of helper file and cause merge conflicts for future bug fixes.

Copy link
Contributor Author

@vivekrnv vivekrnv Aug 13, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If these are moved back to portsyncd.cpp, i won't be able to test those since i can't link my test binary with portsyncd.cpp. Because with gtest we already link it with -lgtest_main so the linker wouldn't allow another main() function to be present in any other files. Thus i need a seperate file to test handlePortConfigFromConfigDB method and g_portSet

@vivekrnv
Moved the fun/global vars to orig location and removed tests
41d31e8 
Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
@vivekrnv
Minor deviations
94b9e30 
Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
@vivekrnv
Minor deviations
aa8e7e7 
Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
@vivekrnv
Copy link
Contributor Author

@prsunny, reviewed the comments discussed offline. Please review

@dgsudharsan dgsudharsan merged commit 8cd38b7 into sonic-net:master Sep 1, 2022
@dprital dprital mentioned this pull request Sep 1, 2022
Closed
7 tasks
prsunny pushed a commit that referenced this pull request Sep 2, 2022
@vivekrnv @prsunny
[UT] [Portsyncd] Added Unit Tests for portsyncd (#2297)
ac8a83f
@DavidZagury DavidZagury mentioned this pull request Sep 14, 2022
Closed
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@prsunny prsunny prsunny approved these changes

@keboliu keboliu Awaiting requested review from keboliu

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@vivekrnv @liat-grozovik @mssonicbld @prsunny @keboliu @dgsudharsan